Skip to content

fix: correct transport request-body handling for body-less and unbufferable streaming bodies#82

Merged
OmarAlJarrah merged 2 commits into
mainfrom
fix/transport-request-body-handling
Jun 16, 2026
Merged

fix: correct transport request-body handling for body-less and unbufferable streaming bodies#82
OmarAlJarrah merged 2 commits into
mainfrom
fix/transport-request-body-handling

Conversation

@OmarAlJarrah

Copy link
Copy Markdown
Member

Summary

Fixes two defects in how the transports adapt request bodies.

1. OkHttp transport rejects a body-less POST/PUT/PATCH

The SDK request model treats a body as optional for every method, so a body-less POST (or PUT/PATCH) is a valid, constructible request. The OkHttp adapter passed a null body straight to Request.Builder.method, which throws IllegalArgumentException("method POST must have a request body.") for the methods OkHttp treats as requiring one. That exception is unchecked, so it escapes the transport's IOException handling and bypasses the retry pipeline — and it diverges from the JDK transport, which already substitutes an empty body for this case.

The adapter now substitutes a shared, zero-length request body for POST/PUT/PATCH when the SDK request carries no body; other methods keep their null body. An explicit method check is used rather than OkHttp's internal HttpMethod symbol to avoid coupling the transport to an unstable cross-module API (the SDK's method enum cannot produce OkHttp's two WebDAV tokens, so POST/PUT/PATCH is the exact reachable set).

2. JDK transport re-drives an already-consumed body on a buffering failure

When streamingPublisher failed to buffer a non-replayable body into a replayable copy (toReplayable() threw mid-write), its fallback called writeTo a second time on the same body. Consume-once bodies flip their guard before writing, so the second writeTo tripped that guard and threw IllegalStateException, masking the original IOException and escaping the method's @Throws(IOException) contract. The "partial bytes" the fallback comment claimed to emit had already been discarded.

The fallback no longer re-drives the body. It fails loudly with an UncheckedIOException that wraps the original IOException and explains that the streaming body was partially consumed and a replayable body is required.

Tests

  • OkHttp: body-less POST, PUT, and PATCH dispatch successfully with an empty recorded body (previously threw IllegalArgumentException).
  • JDK: a non-replayable streaming body whose writeTo fails mid-write now surfaces an UncheckedIOException carrying the original cause, not an IllegalStateException.

Internal change only — no public API change.

Closes #21
Closes #16

…erable bodies

The SDK's Request model treats a request body as optional for every method, but
the two transports mishandled two edge cases of that contract.

OkHttp transport: a body-less POST/PUT/PATCH crashed. RequestAdapter passed a
null body straight to okhttp3.Request.Builder.method, which throws
IllegalArgumentException for the methods OkHttp treats as requiring a body. A
body-less POST is a valid, publicly-constructible SDK request, so this turned a
normal request into an unchecked exception that bypassed the pipeline and diverged
from the JDK transport (which already substitutes an empty body). The adapter now
substitutes a shared zero-length body for POST/PUT/PATCH when no body is present;
other methods keep their null body. An explicit three-token check is used rather
than OkHttp's internal HttpMethod.requiresRequestBody: that symbol is internal to
OkHttp's Kotlin module, and the SDK's Method enum cannot produce the WebDAV tokens
(PROPPATCH/REPORT) that distinguish the two sets.

JDK transport: a non-replayable streaming body whose writeTo failed mid-write
surfaced the wrong exception. When toReplayable() threw, streamingPublisher fell
back to bufferToByteArray, which called writeTo a second time. The SDK's
consume-once bodies flip their consumed guard before writing, so the second
writeTo tripped that guard and threw IllegalStateException, masking the original
IOException and escaping the @throws(IOException) contract on both execute paths.
The partial bytes from the first attempt were already gone. The catch now rethrows
the original IOException wrapped in an UncheckedIOException with a clear message
instead of re-driving the consumed body.

Adds MockWebServer coverage for body-less POST/PUT/PATCH on the OkHttp transport
and a non-replayable mid-write-failure body on the JDK transport.
When toReplayable() fails mid-write while buffering a non-replayable streaming body, the JDK transport now rethrows a checked IOException wrapping the original cause, instead of an UncheckedIOException.

The body is already partially consumed and unrecoverable at that point, so the request must fail — but it is an I/O failure and belongs in execute()'s @throws(IOException) contract. Throwing it unchecked bypassed that contract and diverged from the eager (<=64 KiB) path, which already propagates a mid-write failure as an IOException. The retry pipeline gates on isReplayable(), so a non-replayable body is never re-driven regardless of exception type.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

1 participant